Conversation
Signed-off-by: Jérôme Lafréchoux <jerome@jolimont.fr>
lafrech
requested review from
ArtificialOwl,
icewind1991,
salmart-dev and
sorbaugh
and removed request for
a team
|
I don't think this is a great idea in term of security. What do you think @nextcloud/security ? |
|
All the other files are group-writable so anyone in the owning group can do pretty much anything already. (At least in my own installation, but I think this is standard.) Depending on the installation, the owning group should be www-data, the user's default group, or a dedicated group created on purpose. But I'm interested in the security team's POV, as I have no expertise in Nextcloud administration. |
nickvergessen
left a comment
nickvergessen
left a comment
There was a problem hiding this comment.
✔️ My local dev environment has the same patch applied since a long time 🙈
I hesitated to send it as a patch, as it feels off.
All the other files are group-writable so anyone in the owning group can do pretty much anything already. (At least in my own installation, but I think this is standard.)
That is not recommended and also documented like that in step 13 of the update manual:
https://docs.nextcloud.com/server/stable/admin_manual/maintenance/manual_upgrade.html#step-by-step-manual-upgrade
But long story short, we can do the same trick as with the log file and add a config that has the permissions:
server/config/config.sample.php
Lines 1088 to 1093 in dd6a947
Yes. I didn't know about |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
@lafrech you want to do that change? Or should we take over? |
|
The scope of the change appears to be wider than I expected, and I'm not familiar with the codebase. You may take over if you agree with the feature. Thanks. |
3 participants
Summary
This changes makes config.php group-writable.
This is useful when Nextcloud is installed in a directory with ownership shared between webserver (www-data) and a user with FTP or SSH access, so that the installation/update script may be run as user (CLI) without Nextcloud failing due to www-data not being able to write to the file, or as www-data (from web interface) without the user being unable to move/delete the file from their own cloud space (e.g. to delete Nextcloud).
One may use the setgid bit to ensure all files are owned by a common group, or ACL to ensure user and www-data can read/write all files, but the current chmod break that.
This change keeps the config not readable by others, as specified in the comment.
(See https://help.nextcloud.com/t/installing-nextcloud-in-user-home-directory-on-shared-server/238081/3.)
Checklist
3. to review, feature component)stable32)